Skip to content

Avoid copying Activity tags and links when sampling - #135277

Merged
tarekgh merged 3 commits into
dotnet:mainfrom
martincostello:gh-135007
Oct 7, 2026
Merged

tarekgh merged 3 commits into
dotnet:mainfrom
martincostello:gh-135007

Conversation

@martincostello

Copy link
Copy Markdown
Member

Do not copy links and tags when creating a new Activity when ActivitySamplingResult.PropagationData is specified.

Fixes #135007.

Benchmarks

BenchmarkDotNet, default job, --inProcess --affinity 4095, creating, starting and stopping an activity with the listener returning the given sampling result. main vs. PR; the tag set is the 9 tags ASP.NET Core-style server instrumentation passes at creation, plus one link. Ratios are PR / main (lower is better).

Sampling Creation data Method main Mean main Allocated PR Mean PR Allocated Time Ratio Alloc Ratio
PropagationData None NoTags 120.8 ns 416 B 122.0 ns 416 B 1.01 1.00
PropagationData 9 tags NineTags 211.2 ns 848 B 116.0 ns 416 B 0.55 0.49
PropagationData 9 tags + 1 link NineTagsAndLink 234.7 ns 976 B 116.0 ns 416 B 0.49 0.43
AllDataAndRecorded None NoTags 126.7 ns 416 B 116.4 ns 416 B 0.92 1.00
AllDataAndRecorded 9 tags NineTags 202.0 ns 848 B 195.0 ns 848 B 0.97 1.00
AllDataAndRecorded 9 tags + 1 link NineTagsAndLink 242.9 ns 976 B 225.4 ns 976 B 0.93 1.00
Benchmark Code
using System.Diagnostics;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(PropagationDataBenchmarks).Assembly).Run(args);

[MemoryDiagnoser]
public class PropagationDataBenchmarks
{
    private static readonly KeyValuePair<string, object?>[] s_tags =
    [
        new("client.address", "192.0.2.1"), new("network.peer.address", "192.0.2.1"), new("network.peer.port", 50000),
        new("server.address", "localhost"), new("server.port", 5000), new("http.request.method", "GET"),
        new("user_agent.original", "Mozilla/5.0"), new("url.scheme", "https"), new("url.path", "/api/items"),
    ];

    private static readonly ActivityLink[] s_links = [new(new ActivityContext(ActivityTraceId.CreateRandom(), ActivitySpanId.CreateRandom(), ActivityTraceFlags.None))];

    private ActivitySource _source = null!;
    private ActivityListener _listener = null!;
    private ActivitySamplingResult _result;

    [Params(ActivitySamplingResult.PropagationData, ActivitySamplingResult.AllDataAndRecorded)]
    public ActivitySamplingResult Sampling { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        Console.WriteLine("Assembly: " + typeof(Activity).Assembly.Location);
        _result = Sampling;
        _source = new ActivitySource("Bench");
        _listener = new ActivityListener
        {
            ShouldListenTo = s => ReferenceEquals(s, _source),
            Sample = (ref _) => _result,
        };
        ActivitySource.AddActivityListener(_listener);
    }

    [GlobalCleanup]
    public void Cleanup() { _listener.Dispose(); _source.Dispose(); }

    [Benchmark(Baseline = true)]
    public void NoTags() => Run(null, null);

    [Benchmark]
    public void NineTags() => Run(s_tags, null);

    [Benchmark]
    public void NineTagsAndLink() => Run(s_tags, s_links);

    private void Run(KeyValuePair<string, object?>[]? tags, ActivityLink[]? links)
    {
        var a = _source.CreateActivity("Request", ActivityKind.Server, default(ActivityContext), tags, links);
        a!.Start();
        a.Stop();
    }
}

Do not copy links and tags when creating a new `Activity` when `ActivitySamplingResult.PropagationData` is specified.

Fixes dotnet#135007.
@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Oct 6, 2026
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @steveisok, @dotnet/area-system-diagnostics-tracing
See info in area-owners.md if you want to be subscribed.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @dotnet/area-system-diagnostics-activity
See info in area-owners.md if you want to be subscribed.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Sampler tags violate their documented contract, and the behavioral change needs compatibility documentation.

Review effort: Balanced
Findings: 1 Low severity

Open (1)
What changed in this PR

Optimizes Activity creation for PropagationData sampling.

Changes:

  • Skips copying creation tags, links, and sampler tags.
  • Adds coverage for parent formats and multiple listeners.
File Description
Activity.cs Conditionally skips data copying.
ActivitySourceTests.cs Tests the revised sampling behavior.

@tarekgh tarekgh added this to the 12.0.0 milestone Oct 6, 2026
- Harden check for skipping tags.
- Don't skip sampler tags.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Public API documentation remains inaccurate for the new sampling semantics.

Review effort: Balanced
Findings: 2 Low severity

Open (2)

Update `ActivitySource,CreateActivity()` parameter documentation for `tags` and `links` for new behaviour.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

The existing unresolved compatibility-documentation requirement blocks approval despite the implementation and tests appearing sound.

Review effort: Balanced
Findings: 2 Low severity

Open (2)

@tarekgh tarekgh added the breaking-change Issue or PR that represents a breaking API or functional change over a previous release. label Oct 7, 2026
@dotnet-policy-service dotnet-policy-service Bot added the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 7, 2026
@dotnet-policy-service

dotnet-policy-service Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

  1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
  2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

@tarekgh tarekgh left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tarekgh
tarekgh merged commit 496f6f8 into dotnet:main Oct 7, 2026
101 checks passed
@tarekgh
tarekgh deployed to copilot-pat-pool October 7, 2026 16:42 — with GitHub Actions Active
@tarekgh
tarekgh deployed to copilot-pat-pool October 7, 2026 16:42 — with GitHub Actions Active
@tarekgh
tarekgh deployed to copilot-pat-pool October 7, 2026 16:43 — with GitHub Actions Active
@martincostello
martincostello deleted the gh-135007 branch October 7, 2026 16:44
@tarekgh
tarekgh deployed to copilot-pat-pool October 7, 2026 16:46 — with GitHub Actions Active
@tarekgh
tarekgh deployed to copilot-pat-pool October 7, 2026 16:48 — with GitHub Actions Active
@tarekgh
tarekgh deployed to copilot-pat-pool October 7, 2026 16:48 — with GitHub Actions Active
@github-actions

github-actions Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Breaking Change Documentation

A breaking change draft has been prepared for this PR.

👉 Click here to create the issue in dotnet/docs

After creating the issue, please email a link to it to the
.NET Breaking Change Notifications alias (dotnetbcn@microsoft.com).

Note

This documentation was generated with AI assistance from Copilot.

Generated by Breaking Change Documentation · gpt56 · 44 AIC · ⊞ 27.7K · ◷

@tarekgh

tarekgh commented Oct 7, 2026

Copy link
Copy Markdown
Member

Breaking change issue dotnet/docs#56340 is filed.

@tarekgh tarekgh removed the needs-breaking-change-doc-created Breaking changes need an issue opened with https://github.com/dotnet/docs/issues/new?template=dotnet label Oct 7, 2026

This branch was successfully deployed

1 active deployment
copilot-pat-pool — abc422c1 Deployed Oct 7, 2026 by tarekgh via conclusion #9803
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Diagnostics.Activity breaking-change Issue or PR that represents a breaking API or functional change over a previous release. community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ActivitySource copies creation tags and links into activities sampled as PropagationData

3 participants